Skip to content

Add checks for whitespaces and executable scripts - #543

Draft
vxgmichel wants to merge 3 commits into
masterfrom
vml/add-some-prek-builtin-hooks
Draft

Add checks for whitespaces and executable scripts#543
vxgmichel wants to merge 3 commits into
masterfrom
vml/add-some-prek-builtin-hooks

Conversation

@vxgmichel

Copy link
Copy Markdown
Contributor

This PR adds the following builtin prek hooks to the pre-commit configuration and the CI code checker job:

It also fixes a couple of trailing spaces and executable scripts issues that have been detected by those hooks.


Context

I ran all the builtin prek hooks on the repository to see if any of them was failing. This is the full list:

- trailing-whitespace # Trim trailing whitespace
- check-added-large-files # Prevent committing large files
- check-case-conflict # Check for files that would conflict in case-insensitive filesystems
- check-illegal-windows-names # Check for filenames invalid on Windows
- end-of-file-fixer # Ensure newline at EOF
- file-contents-sorter # Sort lines in explicitly targeted files
- fix-byte-order-marker # Remove UTF-8 byte order marker
- check-json # Validate JSON files
- check-json5 # Validate JSON5 files
- pretty-format-json # Pretty format JSON files
- check-toml # Validate TOML files
- check-vcs-permalinks # Check that VCS links are permalinks
- check-yaml # Validate YAML files
- check-xml # Validate XML files
- mixed-line-ending # Normalize or check line endings
- check-symlinks # Check for broken symlinks
- destroyed-symlinks # Detect destroyed symlinks
- check-merge-conflict # Check for merge conflicts
- detect-private-key # Detect private keys
- no-commit-to-branch # Prevent committing to protected branches
- check-shebang-scripts-are-executable # Ensures that (non-binary) files with a shebang are executable
- check-executables-have-shebangs # Ensures that (non-binary) executables have a shebang

It turns out that 2 of them failed:

Since this is evidence that those issues do occur, I suggest to integrate those checks, along with two similar hooks:

Impact

Those hooks are fast (less than 100ms), and the extra CI job should not be costly either.

vxgmichel added 3 commits May 20, 2026 12:57
Signed-off-by: Vincent Michel <vincent.michel@vates.tech>
Signed-off-by: Vincent Michel <vincent.michel@vates.tech>
Signed-off-by: Vincent Michel <vincent.michel@vates.tech>
@vxgmichel
vxgmichel marked this pull request as ready for review May 21, 2026 07:58
@vxgmichel
vxgmichel requested review from a team as code owners May 21, 2026 07:58
Comment thread .github/workflows/code-checkers.yml
@olivierh-pro
olivierh-pro requested a review from a team May 21, 2026 08:04
@olivierh-pro
olivierh-pro requested a review from a team May 26, 2026 15:07
@vxgmichel
vxgmichel marked this pull request as draft July 2, 2026 15:25
@vxgmichel

Copy link
Copy Markdown
Contributor Author

Thanks @Millefeuille42 and @olivierh-pro for the review

However, #562 made it clear that we need a non-modifying version of the hooks we use so that they can be integrated in the Makefile.

The problem is that the trailing-whitespace and end-of-file-fixer hooks do not provide a non-modifying version.

I'm not sure how to deal with this, I'll have to bring the subject to the team. I switched the PR to draft in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants